Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Allow adding the same dataset multiple times to a bouquet. #507

Merged
merged 11 commits into from
Sep 12, 2024

Conversation

narduin
Copy link
Contributor

@narduin narduin commented Aug 23, 2024

  • J'ai autorisé la sélection du même jeu de données dans un seul bouquet.
  • J'ai cependant ajouté un avertissement lors de la sélection afin que l'utilisateur identifie facilement les datasets déjà utilisés. Initiative personnelle, à voir si ça vous plaît !

image

  • J'ai également supprimé la prop max-height du Multiselect car ça causait un overflow lors de la sélection:

Screenshot from 2024-08-23 16-55-57

  • J'ai dû forcer l'icône du badge car il allait la chercher ici ../../icons/system/fr--warning-fill.svg et ne la trouvait pas.

Fix ecolabdata/ecospheres#206
fix ecolabdata/ecospheres#233

…warning on selection.

Removed max-height on Multiselect.
Forced badge icon from node_modules
@narduin narduin added the ecospheres Spécifique à Ecosphères label Aug 23, 2024
@narduin narduin requested a review from abulte August 23, 2024 15:20
Copy link

netlify bot commented Aug 23, 2024

Deploy Preview for meteo-france ready!

Name Link
🔨 Latest commit 768ad3e
🔍 Latest deploy log https://app.netlify.com/sites/meteo-france/deploys/66e1abe2c056290008f78df9
😎 Deploy Preview https://deploy-preview-507--meteo-france.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Aug 23, 2024

Deploy Preview for ecospheres ready!

Name Link
🔨 Latest commit 768ad3e
🔍 Latest deploy log https://app.netlify.com/sites/ecospheres/deploys/66e1abe290b63600088365ca
😎 Deploy Preview https://deploy-preview-507--ecospheres.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Contributor

@abulte abulte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merci !

J'ai un petit doute sur le "Déjà sélectionné". C'est effectivement une info qui peut être intéressante mais :

  • le warning peut laisser à penser que c'est un problème, hors ce n'est pas franchement le cas
  • je ne sais pas si le message est suffisamment compréhensible, i.e. raccourci de "Déjà sélectionné dans ce bouquet" (un peu long pour un badge)

Alternatives :

  • oublier cet avertissement
  • utiliser un texte à côté de la carte du jeu de données, ce qui nous laisserait plus de place pour mieux expliquer

Si on reste là-dessus, on peut essayer de reprendre le placement du badge "Archivé" sur une carte jeu de données data.gouv.fr https://github.com/datagouv/udata-front/blob/9addd38e9523ff2e4898525a2787938c743f59f7/udata_front/theme/gouvfr/datagouv-components/src/components/DatasetCard/DatasetCard.vue#L3. Le badge actuel me parait un peu à l'étroit à sa place actuelle.

image

J'ai dû forcer l'icône du badge car il allait la chercher ici ../../icons/system/fr--warning-fill.svg et ne la trouvait pas.

Voir la solution data.gouv.fr plus haut pour un contournement (badge DSFR natif). Sinon il faut a minima ouvrir une issue sur vue-dsfr, voire faire une PR si possible.

J'ai également supprimé la prop max-height du Multiselect car ça causait un overflow lors de la sélection:

Est-ce que ça règle ecolabdata/ecospheres#233 ?

@abulte abulte requested a review from streino August 26, 2024 07:42
@streino
Copy link
Contributor

streino commented Aug 26, 2024

J'aime bien l'idée d'avertir l'utilisateur à la sélection, mais d'accord avec @abulte qu'un warning n'est pas le plus approprié. Peut-être le transformer en "info" ?

+1 aussi sur l'emplacement du badge comme "Archivé" si possible.

Pour ce qui est du texte, "Déjà sélectionné dans ce bouquet" serait effectivement plus explicite si tu arrives à le caser. Si on a vraiment la place, on pourrait même envisager d'indiquer sous quel(s) libellé(s) le dataset est déjà présent, pour que l'utilisateur puisse retrouver la/les autres occurrences plus facilement. Autre possibilité moins ergonomique : conserver "Déjà sélectionné" et ajouter un hover/infobulle avec une explication ?

Si on a pas de bonne solution rapide, on peut aussi se contenter du minimum dans cette PR (fix ecolabdata/ecospheres#206) et revisiter le sujet après avoir calé les nouvelles orientations design avec Thomas.

@narduin
Copy link
Contributor Author

narduin commented Aug 26, 2024

Effectivement, un badge info est plus adapté qu'un warning.

  • oublier cet avertissement

Ça me semble important d'avoir cette info pour éviter les erreurs de saisie. Pour le wording j'ai comme alternative: "Déjà présent" ou "Déjà utilisé" qui sont plus courts. À voir s'ils sont assez compréhensibles aussi.

image

Il y a la possibilité de faire une ellipse automatique sur les petits écrans:
image

Est-ce que ça règle ecolabdata/ecospheres#233 ?

J'ai l'impression oui, j'ai testé firefox/chromium sur différentes tailles d'écrans ça a l'air ok.

@streino
Copy link
Contributor

streino commented Aug 26, 2024

Est-ce que ça règle ecolabdata/ecospheres#233 ?

J'ai l'impression oui, j'ai testé firefox/chromium sur différentes tailles d'écrans ça a l'air ok.

Top ! Tu peux l'ajouter en "Fix #..." dans la description du ticket ✌️

Pour le wording j'ai comme alternative: "Déjà présent" ou "Déjà utilisé" qui sont plus courts.

"sélectionné" ou "utilisé" (actifs) me semblent plus clair que "présent" (passif).

Il y a la possibilité de faire une ellipse automatique sur les petits écrans

C'est un peu moche mais ça a le mérite d'être plus clair. On peut partir là dessus en attendant une review plus génerale du design. Ping @thomas-duport pour avis ?

@narduin
Copy link
Contributor Author

narduin commented Aug 26, 2024

J'ai changé pour "utilisé" et ça rentre en 320px. En revanche, ça se superpose à "Entrée pour sélectionner" qui se superpose lui-même d'habitude au titre du jeu de données (peut-être une issue à créer ?)

image

@streino
Copy link
Contributor

streino commented Aug 26, 2024

-> ecolabdata/ecospheres#294

Le mieux serait de corriger 294 d'abord puis rebaser cette PR sur le fix pour ne pas avoir de pb d'overlap. Ca te semble jouable ?

@narduin
Copy link
Contributor Author

narduin commented Aug 26, 2024

-> ecolabdata/ecospheres#294

Le mieux serait de corriger 294 d'abord puis rebaser cette PR sur le fix pour ne pas avoir de pb d'overlap. Ca te semble jouable ?

Oui carrément je me mets dessus !

Copy link
Contributor

@abulte abulte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deux choses :

  • j'ai une erreur 400 via l'API quand j'essaie d'ajouter un jeu de données en double, je pense que c'est parce que les identifiants de jeux de données sont doublonnées dans la liste Topic.datasets, ce que data.gouv.fr ne supporte pas
  • il faudrait traiter le cas de l'ajout via la modale "Ajouter à un bouquet" depuis une page jeu de données, aujourd'hui je ne peux pas ajouter un jeu de données en double dans un bouquet par ce biais
Capture d’écran 2024-09-02 à 17 47 03

@narduin
Copy link
Contributor Author

narduin commented Sep 3, 2024

Deux choses :

* j'ai une erreur 400 via l'API quand j'essaie d'ajouter un jeu de données en double, je pense que c'est parce que les identifiants de jeux de données sont doublonnées dans la liste `Topic.datasets`, ce que data.gouv.fr ne supporte pas

Effectivement, l'ajout en base ne fonctionne pas… En revanche, en ajoutant le même DS depuis la page dudit DS, on peut l'ajouter plusieurs fois. Exemple ici avec le DS-2 et 2-bis.
Par contre on ne peut plus éditer le bouquet, ça le fait complètement planter.

* il faudrait traiter le cas de l'ajout via la modale "Ajouter à un bouquet" depuis une page jeu de données, aujourd'hui je ne peux pas ajouter un jeu de données en double dans un bouquet par ce biais

J'ai ajouté la possibilité de sélectionner un bouquet qui utilise déjà le DS. J'ai rajouté le badge d'information en dessous une fois la sélection faite.

@narduin narduin changed the title Allowed adding the same dataset multiple times to a bouquet. feat: Allow adding the same dataset multiple times to a bouquet. Sep 4, 2024
@abulte
Copy link
Contributor

abulte commented Sep 5, 2024

j'ai une erreur 400 via l'API quand j'essaie d'ajouter un jeu de données en double, je pense que c'est parce que les identifiants de jeux de données sont doublonnées dans la liste Topic.datasets, ce que data.gouv.fr ne supporte pas

pour préciser : il faut donc dédoublonner la liste avant de l'envoyer à data.gouv.fr, a priori ici

datasets: datasetsProperties.value

@narduin
Copy link
Contributor Author

narduin commented Sep 11, 2024

pour préciser : il faut donc dédoublonner la liste avant de l'envoyer à data.gouv.fr

Ça fonctionne ! Ajout depuis la vue bouquet ou depuis la modal du dataset. Le bouquet est bien enregistré et conserve les datasets utilisés plusieurs fois.

À priori pas d'erreur côté API 👍

Copy link
Contributor

@abulte abulte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

En effet, ça marche !

Il faudrait juste changer le texte ici

:text="`Choisissez parmi les ${topicsName}s dont vous êtes l'auteur. Si un ${topicsName} apparait désactivé, c'est que le jeu de données y est déjà associé.`"
pour enlever Si un ${topicsName} apparait désactivé, c'est que le jeu de données y est déjà associé..

@narduin narduin merged commit 7c4cdc9 into main Sep 12, 2024
7 checks passed
@narduin narduin deleted the feat/merge/allow-adding-same-dataset-to-bouquet branch September 12, 2024 08:30
narduin added a commit that referenced this pull request Sep 16, 2024
* Allow adding the same dataset multiple times to a bouquet. Add a warning on selection.
Remove max-height on Multiselect.
FIXME Forced badge icon from node_modules

* Move keyboard instructions down
Prevents it from overlaping on small screens

* visually hide keyboard instruction
Still available to screen readers

* adjust styles of badge

* Move badge intro card component

* Add same DS to bouquet from DS detail

* Deduplicate datasets before updating bouquet

* cleanup import and text
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ecospheres Spécifique à Ecosphères
Projects
None yet
3 participants